Skip to content

SQLFetch & SQLGetData Implementation - #55

Merged
alinaliBQ merged 18 commits into
apache-odbcfrom
sqlfetch-sqlgetdata
Jul 4, 2025
Merged

SQLFetch & SQLGetData Implementation#55
alinaliBQ merged 18 commits into
apache-odbcfrom
sqlfetch-sqlgetdata

Conversation

@alinaliBQ

Copy link
Copy Markdown

SQLFetch & SQLGetData Implementation + Tests

alinaliBQ added 14 commits June 26, 2025 13:40
Remove JSON and struct code
Initial draft code SQLExecDirect

+ Example initial test

SQLGetStmtAttr stub implementation

Stub call for SQLGetStmtAttr

Enable statement handle allocation

Implement SQLExecDirect - test hang issue

Update odbc_api.cc

Run `ShutdownProtobufLibrary` after all ODBC tests

Resolves test hanging problem

Fix missing break statement in SQLGetDiagRec

Add tests

Lint fixes

Run ShutdownProtobufLibrary as part of test environment

Add debug messages

Draft code

Remove drafts

Add comment

Add comment for apacheGH-46889

Update statement test headers

Pass call options to executed prepared statement

`call_options_` contains the authentication token which is needed

SQLFetch & SQLGetData initial impl

EVERYTHING before this commit is for SQLExecDirect

SQLFetch and SQLGetData TestSQLExecDirectSimpleQuery test

Make GetData() return SQLRETURN

`TestSQLExecDirectDataQuery` for remote and mock servers

Remove unneeded test case

Add more data types
Add varbinary test
Will leave tests for later
The `TestSQLExecDirectDataQuery` checks for date is not working.
@alinaliBQ alinaliBQ closed this Jul 3, 2025
@alinaliBQ alinaliBQ reopened this Jul 3, 2025
}

void ODBCStatement::getRowCount(SQLLEN* rowCountPtr) {
if (!rowCountPtr) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see in the spec this error mentioned, which means we can actually just return success and not do anything. Same with num result cols.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. I have removed the thrown errors for SQLNumResultCols and SQLRowCount

throw DriverException("Invalid null pointer for number of column results.", "HY000");
}
size_t columnCount = m_currentArd->GetRecords().size();
*columnCountPtr = columnCount;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to static_cast<> here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, fixed

/// \brief Get invalid connection string for mock server
std::string getInvalidConnectionString() override;
/// \brief Return a SQL query that selects all data types
std::wstring getQueryAllDataTypes() override;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to repeat these tests but use SQL_C_DEFAULT as the target type instead of the real SQL_C_* type. (Real applications like Excel use SQL_C_DEFAULT).

We should also test SQL_ARD_TYPE when descriptor functions are added such as SQLSetDescField.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to validate that the indicator pointer gets set to the correct length (in bytes) for string and binary data. The driver is also allowed to return SQL_NO_TOTAL.

We need to check if a NULL cell correctly sets the indicator pointer to SQL_NULL_DATA.

We should handle test the case where the indicator pointer itself is null.

  • If the cell is non-NULL this should succeed gracefully.
  • If the cell is NULL there's an error that should get reported.

Test that when retrieving fixed-length data, if buffer length is set to a garbage value (like a negative number) it gets ignored.

We also need to test retrieving string and binary data in parts. When the buffer length is too small:

  • Return SQL_SUCCESS_WITH_INFO with error state 01004
  • I think the indicator pointer is supposed to hold the length of the remaining data after the current chunk. If the indicator pointer is NULL this should behave normally.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, today I have added tests for everything you mentioned except case:

Test that when retrieving fixed-length data, if buffer length is set to a garbage value (like a negative number) it gets ignored.

Will continue working on this on Friday

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added test for passing garbage value as buffer length

- do not return errors for invalid rowCountPtr and columnCountPtr
- add static cast for columnCount
- Add tests with SQL_C_DEFAULT as target type
- Add test checks for indicator
- Fix SQL_NUMERIC, SQL_BIT, SQL_FLOAT target type

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will continue addressing comments tmr

}

void ODBCStatement::getRowCount(SQLLEN* rowCountPtr) {
if (!rowCountPtr) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. I have removed the thrown errors for SQLNumResultCols and SQLRowCount

throw DriverException("Invalid null pointer for number of column results.", "HY000");
}
size_t columnCount = m_currentArd->GetRecords().size();
*columnCountPtr = columnCount;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, fixed

/// \brief Get invalid connection string for mock server
std::string getInvalidConnectionString() override;
/// \brief Return a SQL query that selects all data types
std::wstring getQueryAllDataTypes() override;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, today I have added tests for everything you mentioned except case:

Test that when retrieving fixed-length data, if buffer length is set to a garbage value (like a negative number) it gets ignored.

Will continue working on this on Friday

@alinaliBQ

Copy link
Copy Markdown
Author

The C++ / AMD64 Conda C++ AVX2 (pull_request) failures should not due to our code but rather something at Arrow's source

@alinaliBQ
alinaliBQ marked this pull request as ready for review July 4, 2025 19:04
@alinaliBQ
alinaliBQ merged commit 9109e6f into apache-odbc Jul 4, 2025
22 of 24 checks passed
@alinaliBQ
alinaliBQ deleted the sqlfetch-sqlgetdata branch July 4, 2025 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants